Compact style mutation #464

eoghanmurray · 2021-01-18T13:09:17Z

I had one recording in the wild that turned into a monster at 12GB.
One of the sources was continuous DOM Mutations caused by a 3rd party 'snow animation' library that was animating individual snowflakes multiple times per second via JavaScript, causing a constant stream of very large mutations.

Each mutation was of the format
"style":"color: rgb(255, 255, 255); position: absolute; width: 8px; height: 8px; font-family: arial, verdana; overflow: hidden; font-weight: normal; z-index: 0; display: block; bottom: auto; opacity: 1; padding: 0px; margin: 0px; font-size: 10px; line-height: 10px; text-align: center; vertical-align: baseline; left: 242.807px; top: 85.7332px;"

Even though just the left or top style attributes had changed.
The new format for this part of the attribute mutation looks like:
"style": {"left": "242.807px", "top": "85.7332px"}

~~There's a threshold to the attribute length before this change applies, currently set at a style attribute length of 25 characters (this value could be tweaked)~~ (the threshold would have caused errors, so I've removed it)

Diffs are generated by creating a dummy node to house the old style and then comparing each style (sub)attribute in turn using getPropertyValue and getPropertyPriority. Attributes on the old value are also iterated over to see if any of them have been removed.

At playback time, these sub-style mutations are applied via setProperty with either a single value argument, or 2 arguments in the case that the !important priority flag is present in the change.

eoghanmurray · 2021-01-18T17:47:34Z

(Sorry still iterating on this as tests are not passing; my dev test environment isn't working so uploading changes here to check)

eoghanmurray · 2021-01-19T18:37:55Z

~~@Yuyz0112 would you be able to take a look at the failing build e.g. https://travis-ci.com/github/rrweb-io/rrweb/builds/213675796~~

~~i.e. there should be a text attribute change as follows:~~
~~"style": ""left: 8px; width: 46.2109px; top: 115.5px; bottom: auto; display: block;""~~

[Edit] #468 has the fix for why the style changes weren't showing up [End Edit]

eoghanmurray · 2021-01-20T16:28:43Z

Okay, I've got all the tests sorted now and have added one extra test for !important as this is treated separately.

Aside: during replay, it won't break if a previous string value for the style attribute is present from a prior recording.

Juice10 · 2021-02-19T13:22:41Z

src/record/mutation.ts

+	    const new_value = target.style.getPropertyValue(pname);
+	    const new_priority = target.style.getPropertyPriority(pname);


For code hygiene it's probably better to camelCase these variable names.

Suggested change

const new_value = target.style.getPropertyValue(pname);

const new_priority = target.style.getPropertyPriority(pname);

const newValue = target.style.getPropertyValue(pname);

const newPriority = target.style.getPropertyPriority(pname);

Thanks; I take it I cannot just commit this suggestion as-is?
I'll make a new commit.

Updated the original commit with your suggested change as was force-pushing anyhow.

Juice10 · 2021-07-03T06:27:44Z

Looks good to me, only thing I can think of would be doing some checking in the replay, if the element has a style attribute. Just in case something went wrong with the recorded ids and we are accidentally referencing a text node. I believe there is a try/catch around the get/setAttributes for that reason on the string type of style mutation.

Yuyz0112 · 2021-07-05T05:14:44Z

@eoghanmurray Looks like this approach will not trigger reflow/repaint during diff, because the dummy node is not in the DOM tree, am I right?

And is it worth a little benchmark test to show the time of diff?

eoghanmurray · 2021-07-06T10:31:12Z

Ya not added to the DOM; I felt safe creating the dummy node as we are doing that elsewhere in the code e.g. at https://github.com/rrweb-io/rrweb-snapshot/blob/master/src/snapshot.ts#L204

I made a quick performance.now() comparison and got the following results

Firefox (over 277 events):
Old avg. 0.184ms
New avg 0.234ms
Slower 1.27x

Chrome (over 256 events):
Old avg 0.073ms
New avg 0.224ms
Slower 3.03x

Looks like the difference is due to the existing transformAttribute code path being just as slow in Firefox.

This is happening in the DOMMutation callback which is 'off the main loop', right?

…ngle style properties result in storage of a rewrite for the full style attribute, which may be very large. Had an example of a website using http://schillmania.com/projects/snowstorm/ where many direct style changes were happening every second across many 'snowflake' elements, with each attribute change looking like: "style":"color: rgb(255, 255, 255); position: absolute; width: 8px; height: 8px; font-family: arial, verdana; overflow: hidden; font-weight: normal; z-index: 0; display: block; bottom: auto; opacity: 1; padding: 0px; margin: 0px; font-size: 10px; line-height: 10px; text-align: center; vertical-align: baseline; left: 242.807px; top: 85.7332px;" even though maybe just the left/top position had been changed

… an `!important` flag - saves 6 chars per style attr in the json :)

…t of 'style' attributes

Yuyz0112 · 2021-07-11T12:04:35Z

LGTM

@eoghanmurray This happens off the main loop, but still executing in sync mode, so it may block following execution. I think we can:

support running record logic in web worker
do record logic in request idle callback

eoghanmurray · 2021-07-12T10:05:10Z

for 1. I thought web workers didn't have access to the DOM?
2. looks good where browser supports it

…ge of rrweb-io#464

* My best interpretation of what the typings should look like after merge of #464 * Apply variable name changes as per Juice10 review Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com> * fix types Co-authored-by: Eoghan Murray <eoghan@getthere.ie>

razor-x · 2022-01-27T03:55:20Z

Hello. I'm coming here as a PostHog user. It uses this lib to power session recording. My app is able to run a strict CSP, specifically without 'unsafe-inline' in the policy. However, I noticed a lot of CSP errors generated like the one below:

After some digging, I believe this change broke support for users who run a strict CSP. Setting the style attribute on the dummy node will be blocked without 'unsafe-inline'.

Would you prefer I start a new issue or continue discussion here on possible solutions? Best.

Juice10 · 2022-01-27T06:06:51Z

@razor-x thanks for bringing that issue up!
It probably makes most sense to start a new issue for that.

eoghanmurray force-pushed the compact-style-mutation branch 5 times, most recently from 6031664 to b11673e Compare January 18, 2021 16:24

eoghanmurray force-pushed the compact-style-mutation branch 6 times, most recently from 6304e0e to 69c1e30 Compare January 19, 2021 16:28

eoghanmurray mentioned this pull request Jan 20, 2021

Tests: don't delete style attributes in post-processing #468

Merged

eoghanmurray force-pushed the compact-style-mutation branch from 69c1e30 to 6f686c9 Compare January 20, 2021 16:20

eoghanmurray force-pushed the compact-style-mutation branch from 6f686c9 to 94eb6d6 Compare January 20, 2021 16:34

eoghanmurray force-pushed the compact-style-mutation branch 2 times, most recently from d0c4f0c to b92e85a Compare February 3, 2021 17:53

Juice10 reviewed Feb 19, 2021

View reviewed changes

eoghanmurray force-pushed the compact-style-mutation branch 2 times, most recently from fc68ba7 to 2ddd84f Compare February 22, 2021 12:30

eoghanmurray force-pushed the compact-style-mutation branch 3 times, most recently from 8855836 to 38f84e2 Compare June 15, 2021 12:22

eoghanmurray force-pushed the compact-style-mutation branch 2 times, most recently from a463f69 to b42dda8 Compare June 25, 2021 16:05

eoghanmurray force-pushed the compact-style-mutation branch from b42dda8 to ee7c23f Compare July 2, 2021 20:45

eoghanmurray force-pushed the compact-style-mutation branch from ee7c23f to cbe0914 Compare July 8, 2021 09:15

eoghanmurray added 3 commits July 8, 2021 16:11

More compact storage for the much more common attribute value without…

8307256

… an `!important` flag - saves 6 chars per style attr in the json :)

Fix bug: attributes weren't getting removed after changes to treatmen…

ea78955

…t of 'style' attributes

eoghanmurray force-pushed the compact-style-mutation branch from cbe0914 to ea78955 Compare July 8, 2021 15:11

Yuyz0112 merged commit 9875a3d into rrweb-io:master Jul 11, 2021

eoghanmurray added a commit to statcounter/rrweb that referenced this pull request Jul 13, 2021

My best interpretation of what the typings should look like after mer…

c98b1eb

…ge of rrweb-io#464

razor-x mentioned this pull request Jan 27, 2022

Regression: support for strict CSP broken by style mutation #816

Open

eoghanmurray mentioned this pull request Jul 27, 2023

Compact style mutation fixes and improvements #1268

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compact style mutation #464

Compact style mutation #464

eoghanmurray commented Jan 18, 2021 •

edited

Loading

eoghanmurray commented Jan 18, 2021

eoghanmurray commented Jan 19, 2021 •

edited

Loading

eoghanmurray commented Jan 20, 2021

Juice10 Feb 19, 2021

eoghanmurray Feb 22, 2021

eoghanmurray Feb 22, 2021

Juice10 commented Jul 3, 2021

Yuyz0112 commented Jul 5, 2021

eoghanmurray commented Jul 6, 2021

Yuyz0112 commented Jul 11, 2021

eoghanmurray commented Jul 12, 2021 •

edited

Loading

razor-x commented Jan 27, 2022

Juice10 commented Jan 27, 2022

		const new_value = target.style.getPropertyValue(pname);
		const new_priority = target.style.getPropertyPriority(pname);

Compact style mutation #464

Compact style mutation #464

Conversation

eoghanmurray commented Jan 18, 2021 • edited Loading

eoghanmurray commented Jan 18, 2021

eoghanmurray commented Jan 19, 2021 • edited Loading

eoghanmurray commented Jan 20, 2021

Juice10 Feb 19, 2021

Choose a reason for hiding this comment

eoghanmurray Feb 22, 2021

Choose a reason for hiding this comment

eoghanmurray Feb 22, 2021

Choose a reason for hiding this comment

Juice10 commented Jul 3, 2021

Yuyz0112 commented Jul 5, 2021

eoghanmurray commented Jul 6, 2021

Yuyz0112 commented Jul 11, 2021

eoghanmurray commented Jul 12, 2021 • edited Loading

razor-x commented Jan 27, 2022

Juice10 commented Jan 27, 2022

eoghanmurray commented Jan 18, 2021 •

edited

Loading

eoghanmurray commented Jan 19, 2021 •

edited

Loading

eoghanmurray commented Jul 12, 2021 •

edited

Loading